Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to new pac #371

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

usbalbin
Copy link

@usbalbin usbalbin commented Dec 4, 2024

This updates this hal to use the stm32f3-staging pac. I am doing this in an attempt to get the HRTIM peripheral to work.

@usbalbin usbalbin mentioned this pull request Dec 4, 2024
13 tasks
@usbalbin
Copy link
Author

I had to enable the feature critical-section of the pac to gain access to the method Peripherals::take() which is used in the examples. Further, an implementation is needed or there was a linker error so I also had to enable critical-section-single-core for cortex-m. The last part, I believe, enabled some extra examples which I do not think the CI has been checking before. This is now the most if not all the errors I am getting in the CI now

@usbalbin usbalbin changed the title [Experiment]Update to new pac Update to new pac Dec 15, 2024
src/dma.rs Show resolved Hide resolved
src/i2c.rs Outdated Show resolved Hide resolved
src/watchdog.rs Outdated

let psc = self.iwdg.pr.read().pr().variant();
let reload = self.iwdg.rlr.read().rl().bits();
let psc = self.iwdg.pr().read().pr().variant().unwrap(); // <--- Unwrap!!
Copy link
Author

@usbalbin usbalbin Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts as to what to do here? Should we provide a default or something? Or can this possibly fail?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like H5 guy has broke this:
stm32-rs/stm32-rs@c812a9e

There should be default 256 value

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks a lot! stm32-rs/stm32-rs#1045

src/dac.rs Outdated Show resolved Hide resolved
@usbalbin
Copy link
Author

Correct me if I am wrong, but I do not think I am guilty to this one

error[E0599]: no method named `split` found for struct `Serial` in the current scope
  --> examples/serial_dma.rs:44:27
   |
44 |     let (tx, rx) = serial.split();
   |                           ^^^^^ method not found in `Serial<USART1, (Pin<Gpioa, U<9>, Alternate<PushPull, 7>>, Pin<Gpioa, U<10>, Alternate<PushPull, 7>>)>`
   |
   = note: the full type name has been written to '/home/runner/work/stm32f3xx-hal/stm32f3xx-hal/target/thumbv7em-none-eabihf/debug/examples/serial_dma-091110dd37752b66.long-type-1484845090031692940.txt'
   = note: consider using `--verbose` to print the full type name to the console

@usbalbin usbalbin marked this pull request as ready for review December 16, 2024 20:01
@usbalbin
Copy link
Author

@burrbull would you like to take a look at this?

src/spi.rs Outdated
Comment on lines 375 to 377
let write_ptr = core::ptr::addr_of!(self.spi.dr) as *mut Word;
let write_ptr = self.spi.dr().as_ptr() as *mut Word;
// SAFETY: Write to register owned by this Spi struct
unsafe { core::ptr::write_volatile(write_ptr, word) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend to use dr register for Word = u16 and dr8 register for Word = u8 so you don't need for casting:

self.spi.dr8().write(|w| dr().set(word));

Same for reading.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like 9cb658e?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

channel::Id::Sixteen => self.reg.smpr2.read().smp16().variant().into(),
channel::Id::Seventeen => self.reg.smpr2.read().smp17().variant().into(),
channel::Id::Eighteen => self.reg.smpr2.read().smp18().variant().into(),
channel::Id::Zero => self.adc.smpr1().read().smp0().variant().into(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you specify channel::Id::Zero as 0 you could write it shorter:

let ch = channel as u8 
if ch < 9 {
    self.adc.smpr1.read().smp(ch).variant().into()
} else {
    self.adc.smpr1.read().smp(ch - 9).variant().into()
}

Copy link
Author

@usbalbin usbalbin Dec 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like this a8f8835?

Edit: I think i got some errors

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They dont seem to be array-ified

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. You are right. I've not finished this yet.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants